Skip to content

[자동차경주] 김경수 2주차 미션 제출합니다#8

Open
Siul49 wants to merge 8 commits intoJava-JavaScript-Language-Stuty:mainfrom
Siul49:main
Open

[자동차경주] 김경수 2주차 미션 제출합니다#8
Siul49 wants to merge 8 commits intoJava-JavaScript-Language-Stuty:mainfrom
Siul49:main

Conversation

@Siul49
Copy link
Copy Markdown

@Siul49 Siul49 commented Feb 14, 2025

리뷰하러 와주신 분들 감사합니다!

오늘하루도 행복하세요~

1. 신경써서 구현한 부분

  • MVC 패턴을 도입했습니다. (최대한 캡슐화에 신경쓰려 했습니다)
  • 저도모르게 static을 마구 붙여서 편하게 하려는 버릇을 고치려 노력했습니다.
  • 변수명을 정하는 것에 신경 많이썼습니다.

2. 피드백이 필요한 부분

  • MVC 패턴을 적용한 것이 처음인지라 더 좋게 구분할만한 요소가 있는지 제대로 적용한 것인지
  • 가독성에도 신경쓰려 변수 명에도 신경을 조금 썼는데 가독성을 해치는 부분
  • 알고리즘의 짜임새도 더 좋게 쓸 수 있을만한 부분
  • 오류가 발생할 법한 부분까지 알려주신다면 정말 감사하겠습니다.

3. 내가 고려한 예외

  • 문자가 5개 이상인 경우

Copy link
Copy Markdown

@seulnan seulnan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

너무 고생하셨습니다:)

pr설명대로 변수명에 정말 신경많이쓰셨네요ㅎㅎ
다만, static이나 캡슐화부분에서는 어떤건 적용하셨는데 어떤 부분은 적용안하신 코드도 보입니다!
appconfig파일을 만들어서 DI 흐름을 정리하는 것은 어떨까용??

추가적으로 오류가 난 부분들은 테스트코드를 많이 작성해보면서 충분히 고칠 수 있다고 생각해요.
갠적으로 저는 디버깅하는 과정에서 많은 걸 배우는 편이라 꼭 오류도 고쳐보셨으면 좋겠습니다 파이팅이요 👊🏻👊🏻

import camp.nextstep.edu.missionutils.Randoms;

public class Car{
public int Distance;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

distance필드를 public으로 선언한 이유가 있을까요?? 캡슐화에 신경쓰셨다고해서용! @Siul49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

field 변수명의 첫 글자는 소문자가 맞는 것 같습니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헛 private선언을 못한 부분이 있군요..!! 제 실수입니다.. :) @seulnan

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

field 변수명의 첫 글자가 소문자여야하는 이유가 따로 있는건가요? @BYEONGHWALEE-dev

StringTokenizer separatedCarNames = new StringTokenizer(CarNames, ",");

while(separatedCarNames.hasMoreTokens()){
if (separatedCarNames.nextToken().trim().length() < 5) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중복코드가 사알짝 보이네용 하필 nextToken이라 두번 호출하면 건너뛰어질 가능성도 있어보여용
nextToken을 한번만 호출한 값을 변수에 저장하고 사용하는 방식으로 수정이 필요해보입니다!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와 맞네요 감사합니당

Comment on lines +25 to +51
public void RacingGame() {
output.CarNameRequestMessage();
input userInput = new input();
userInput.setCarName();

output.trialRequestMessage();
userInput.setTrial();

setCar(userInput.getCarName());

for (int k = 0; k < userInput.getTrial(); k++){
for (int i = 0; i < numbersOfCar; i++) {
int randomDistance = Randoms.pickNumberInRange(0, 9);
car[i].Distance += randomDistance;
}

System.out.print(car[k].getName() + " : ");
for (int i = 0; i < car[i].Distance; i++){
if (i != car[i].Distance){
System.out.print("-");
}
else {
System.out.println("-");
}
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기에 게임진행하는 로직과 출력로직이 같이 섞여있는 것 같아용 util이나 service을 활용해서 분리하는 것은 어떨까용?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 그렇게 하는게 가독성이 더 좋겠네요 감사합니당

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다


setCar(userInput.getCarName());

for (int k = 0; k < userInput.getTrial(); k++){
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 이중 for문으로 인해 가독성이 살짝 떨어지는 것 같은데 다른 API를 쓰지않고 이중 for문을 활용하신 이유가 있을까용?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다. for문을 한번만 써서 시간 복잡도를 줄일 수 있을 것 같습니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API 공부가 조금 부족했던 것 같네요..ㅜㅜ 혹시 여기서 추천해주실만한 API가 있을까요? @seulnan @BYEONGHWALEE-dev

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 어떻게 분리하냐에 따라 달라질 것 같긴하데, 다른 분들 코드처럼 forEach()나 Stream으로 충분히 가능해보여요!

car[i].Distance += randomDistance;
}

System.out.print(car[k].getName() + " : ");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가적으로 바깥 for루프에서는 k를 활용해서 시도횟수를 반복하는것 같고,
안쪽 for루프에서는 i라는 인덱스를 활용하여서 각 자동차를 이동하는 것 같은데
자동차를 출력할때는 그러면 car[k]가 아닌 안쪽 for루프에서 car[i]를 써야하는거아닌가요...?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와 여기 코드 단단히 실수했군요 감사합니다 @seulnan

Copy link
Copy Markdown

@BYEONGHWALEE-dev BYEONGHWALEE-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생 많으셨습니다.


setCar(userInput.getCarName());

for (int k = 0; k < userInput.getTrial(); k++){
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다. for문을 한번만 써서 시간 복잡도를 줄일 수 있을 것 같습니다.

Comment on lines +25 to +51
public void RacingGame() {
output.CarNameRequestMessage();
input userInput = new input();
userInput.setCarName();

output.trialRequestMessage();
userInput.setTrial();

setCar(userInput.getCarName());

for (int k = 0; k < userInput.getTrial(); k++){
for (int i = 0; i < numbersOfCar; i++) {
int randomDistance = Randoms.pickNumberInRange(0, 9);
car[i].Distance += randomDistance;
}

System.out.print(car[k].getName() + " : ");
for (int i = 0; i < car[i].Distance; i++){
if (i != car[i].Distance){
System.out.print("-");
}
else {
System.out.println("-");
}
}
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다.

maxDistance = cars.car[i].Distance;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream API를 통해 코드를 더 간결하게 짤 수 있을 것 같아요!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다

import camp.nextstep.edu.missionutils.Randoms;

public class Car{
public int Distance;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

field 변수명의 첫 글자는 소문자가 맞는 것 같습니다.

import java.util.ArrayList;

public class RacingCarController{
public int numbersOfCar = 1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변수를 private 접근지정자로 하면 더 좋을 것 같아요!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다

else {
System.out.println("-");
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"-" 출력을 하나로 합치고 마지막에 '\n'을 출력하는 방법은 어떨까요!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의합니다! indent depth를 2까지 허용한다고 했는데 지금은 3인 것 같아요!

System.out.print(winner.getFirst());
winner.removeFirst();
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이부분도 겹치는 출력 부분을 하나로 합치는게 나을 것 같아요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants